Fix file dialog crash and drive list on Linux - #279
Merged
Conversation
Fixes #273. Opening any of the file dialogs threw as soon as the listed directory held two entries of the same kind: System.InvalidOperationException: Failed to compare two elements in the array. ---> System.ArgumentException: Object must be of type String. DrawFileTable sorted with .ThenBy(p => p) over a Collection<IAbsolutePath>. That interface carries no IComparable<T>, so LINQ falls back to Comparer<T>.Default, which calls the non-generic IComparable.CompareTo(object) on the underlying semantic string — and that overload only accepts a System.String, not another semantic string. Sorting now uses the entry's string value, which also gives case-insensitive display order with a deterministic tiebreak. Reproduced with an identical stack outside the UI; despite the issue title it is not Linux-specific. DrawDrivesCombo was broken in three ways, two of them Linux-only: - The preview label was hardcoded to Drives[0], so browsing D:\ on Windows still displayed C:\. It now shows the current root. - The "current drive" was built from Path.VolumeSeparatorChar, which is '/' on Unix rather than ':', so /home/matt produced garbage like matt// and no entry ever highlighted. It now compares against Path.GetPathRoot. - Environment.GetLogicalDrives() returns every mount point on Unix — 28 entries on the development machine, mostly /proc, /sys, /dev and /run pseudo filesystems, with duplicates. The list is now filtered, deduplicated and sorted, down to 4 real entries. /run/media is exempt from the /run exclusion because that is where Arch and Fedora mount removable media. Adds tests/ImGui.Popups.Tests covering the sort and the drive logic. Reverting the sort fix fails 4 of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFRwYRHhEmVMGvrH2BbtbW
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #273.
The crash
Every file dialog threw as soon as the listed directory held two entries of the same kind:
DrawFileTablesorted with.ThenBy(p => p)over aCollection<IAbsolutePath>. That interface carries noIComparable<T>, so LINQ falls back toComparer<T>.Default→ObjectComparer→ the non-genericIComparable.CompareTo(object)on the underlying semantic string — an overload that accepts only aSystem.String, not another semantic string.Sorting now uses the entry's string value, which also gives case-insensitive display order with a deterministic tiebreak for names differing only by case.
I reproduced this outside the UI with an identical stack: despite the issue title it is not Linux-specific, it fires on any platform. The reporter simply hit it first.
The same bug is fixed at the root in ktsu-dev/Semantics#140, but this change stands on its own — sorting by the string value avoids boxing every comparison regardless, and
IAbsolutePathstill won't have a generic comparer after that lands.The drives combo
Three separate defects, two Linux-only:
Drives[0]— on Windows with C:/D:, browsingD:\still displayedC:\. Now shows the current root.Path.VolumeSeparatorChar, which is'/'on Unix rather than':', so/home/mattproduced garbage likematt//. Now compares againstPath.GetPathRoot.Environment.GetLogicalDrives()returns every mount point on Unix — 28 entries on my machine, mostly/proc/sys/fs/binfmt_misc,/sys/kernel/debug,/dev/mqueue,/run/user/1000/gvfs, with duplicates. Filtered, deduplicated and sorted, that becomes 4 real entries with the current root highlighted./run/mediais explicitly exempt from the/runexclusion, since that is where Arch and Fedora mount removable media — a blanket filter would have hidden USB drives.Tests
New
tests/ImGui.Popups.Testsproject (13 tests) covering the sort and the drive logic. Reverting the sort fix fails 4 of them. Full suite: 603 passed, 1 skipped (a Windows-only drive assertion), builds clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01WFRwYRHhEmVMGvrH2BbtbW